Merged
Conversation
|
🚅 Deployed to the rivet-pr-4521 environment in rivet-frontend
|
This was referenced Mar 28, 2026
11 tasks
This was referenced Mar 28, 2026
Merged
Member
Author
PR Review: chore: envoy client\n\nBlocking issues before merge:\n\n1. Local link dependency for antiox in package.json hardcodes /home/nathan/antiox. This breaks CI. antiox must be published to npm or added as a workspace package first.\n\n2. destroyActor sends ActorIntentStop, identical to stopActor. The doc says it avoids hibernation but no ActorIntentDestroy exists. Either fix the intent or remove destroyActor from the public API.\n\n3. ToEnvoyTunnelMessage handler is a TODO comment. config.fetch and config.websocket in EnvoyConfig are never called. HTTP and WebSocket routing from the gateway are non-functional.\n\n4. todo.md is committed inside the SDK package. Per CLAUDE.md it belongs in .agent/todo/ or .agent/notes/.\n\nSignificant issues:\n\n5. commandIdx is hardcoded as 0n in sendActorIntent. If meaningful it is a bug; if not, remove it from the type.\n\n6. ACK_COMMANDS_INTERVAL_MS is 5 minutes with no initial send on connect. The server can wait up to 5 minutes for a command ack after reconnect. Consider sending ack immediately after ToEnvoyInit.\n\n7. The timestamp field in kv.ts should follow the project naming convention of ended_at, created_at, etc.\n\nMinor issues:\n\n8. log(ctx) in actor.ts creates a new child logger on every call, unlike connection.ts which caches it.\n9. uint8ArraysEqual in envoy/index.ts duplicates arraysEqual already exported from utils.ts.\n10. kvGet has an O(n^2) key lookup; a hex-keyed Map would fix this.\n11. ws: any in EnvoyConfig.websocket should be a typed interface.\n12. prepopulateActorNames metadata should be Record<string, unknown> not any.\n13. Several comments are not complete sentences per CLAUDE.md style.\n\nPositive aspects:\n\nThe split into focused task modules mirrors the Tokio-style task decomposition from the Rust side and is a large improvement. Correct adoption of antiox primitives replaces ad hoc Promise queues. KV lifecycle and event history with resend-on-reconnect are well designed. The stringify.ts helpers improve debug logging. |
23cce47 to
949143d
Compare
b99d8e5 to
c461b02
Compare
11 tasks
c461b02 to
f902085
Compare
949143d to
afb8d48
Compare
f902085 to
6e4a901
Compare
afb8d48 to
d52568e
Compare
6e4a901 to
056dbbc
Compare
11 tasks
056dbbc to
ecb7a20
Compare
d52568e to
fb6f723
Compare
11 tasks
ecb7a20 to
58851a4
Compare
fb6f723 to
67b71f3
Compare
67b71f3 to
fddb429
Compare
58851a4 to
68bbabd
Compare
11 tasks
fddb429 to
e4d920a
Compare
68bbabd to
5c9a464
Compare
This was referenced Apr 3, 2026
This was referenced Apr 4, 2026
11 tasks
11 tasks
Member
Author
Merge activity
|
5c9a464 to
7526490
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: